[Chore] Error message on missing API key for forks (CF-675)#371
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
…-on-missing-key-in-fork`) Here’s a **faster** version, making these improvements. - Avoid repeatedly calling `os.getenv()` and `Path.open()` for the same event file, by using an explicit cache variable for the file contents. This accelerates repeated lookups and reduces disk accesses. - Remove double @lru_cache use, as the value from `get_cached_gh_event_data()` will not change for a process lifetime and can be memory-cached explicitly. - Microoptimize the `is_repo_a_fork` logic by removing an unnecessary bool(...) coercion (the value is already True/False). Here’s the rewritten program. **Key notes:** - The event file is read at most once per process. - All redundant calls and mutable global state are avoided. - Fast subsequent access due to a module-level cache; you’ll see lower file-system and JSON load latency. - The returned values are identical for the same environment and state, and function signatures are maintained. Let me know if you want further micro-optimizations!
⚡️ Codeflash found optimizations for this PR📄 40% (0.40x) speedup for
|
…hore/error-on-missing-key-in-fork`) Here’s a faster and more memory-efficient version of your program. **Optimization rationale:** - Avoids the extra `Path` object creation and `.open()` call—using `open(event_path)` is faster for a single use. - Reduces dependency usage by not involving `Path` unless file system path manipulation is needed. - Leaves error handling as originally absent, maintaining original logic. - Leaves `lru_cache` as is and all function/return types unchanged. **Summary of change:** Direct file open instead of through `Path`, removing unnecessary abstraction for a single read and reducing overhead. All logic and return values are identical.
| if is_repo_a_fork(): | ||
| msg = ( | ||
| "Codeflash API key not detected in your environment. It appears you're running Codeflash from a GitHub fork.\n" | ||
| "For external contributors, please ensure you've added your own API key to your fork's repository secrets and set it as the CODEFLASH_API_KEY environment variable.\n" | ||
| f"{api_secret_docs_message}" | ||
| ) | ||
| raise OSError(msg) |
There was a problem hiding this comment.
use codeflash/code_utils/code_utils.py::exit_with_message
do we want to exit with an error code since that will mark the GHA run as failed? @misrasaurabh1 thoughts there?
There was a problem hiding this comment.
@mohammedahmed18 meant to request changes here, not approve, make the changes please

User description
PR Type
Enhancement
Description
Enhance API key error messaging for forks
Add fork detection via GITHUB_EVENT_PATH
Include docs link in error message
Import json and adjust type hints
Changes walkthrough 📝
env_utils.py
Fork detection and improved API key errorscodeflash/code_utils/env_utils.py